TypeScript Core#8861
Conversation
| "packageManager": { | ||
| "name": "npm" | ||
| } | ||
| } |
There was a problem hiding this comment.
Very strange choice. How about simply "version": ">= 20"?
There was a problem hiding this comment.
Node.JS has this unusual convention that odd-numbered versions are not stable and become unsupported after 6 months. Only even-numbered versions receive continued long-term support (bugfixes & security updates) after ‘current’ development, which is why i put "^20 || ^22 || ^24 || ^26".
Luckily this is about to change starting in v27 (planned April 2027) where:
Starting with Node.js 27, the release cycle will be annual and every major version will move to LTS status after its six-month Current phase (and six additional months of Alpha phase).
There was a problem hiding this comment.
Setting a minimum version is pretty standard practice. You do it once and then forget about it. In your case, though, you'll need to update the list every time a new major node.js version comes out! And why does it matter whether the version is stable or not stable? It doesn't suddenly break anything.
In addition, there are users who rarely update, while others use only the latest version of node (which may not be included in that list).
There was a problem hiding this comment.
Fair enough, and agreed, it'd be better to set it once and only update once a version reaches end-of-life. I'll set the minimum to >= 22
| target_link_libraries(binaryen_js PRIVATE "-sEXPORT_ES6") | ||
| endif() | ||
| target_link_libraries(binaryen_js PRIVATE "-sEXPORTED_RUNTIME_METHODS=stringToUTF8OnStack,stringToAscii,getExceptionMessage") | ||
| target_link_libraries(binaryen_js PRIVATE "-sEXPORTED_RUNTIME_METHODS=out,err,HEAP8,HEAPU8,HEAP32,HEAPU32,stackSave,stackRestore,stackAlloc,UTF8ToString,stringToAscii,stringToUTF8OnStack,getExceptionMessage") |
There was a problem hiding this comment.
Why we need extended exported symbols? I haven't seen them being used. Also js-bindings works without all of them normally.
There was a problem hiding this comment.
Most of these symbols aren't needed in this PR but they will be needed in future code migration (example links below are from #8826):
outwill be used in emitAsmJS- i can remove
err, looks like it's not used in the TS anywhere - all the heaps are used in various functions, e.g.:
- HEAP8 in DataSegment
- HEAPU8, HEAP32, & HEAPU32 in emitBinary
stackSave,stackRestore, andstackAllocwill be needed in utilitiesUTF8ToString,stringToAscii, andstringToUTF8OnStackfor anything using javascript strings (e.g. parseText)
The reason the JS bindings work without them is because the current post.js file is appended to (i think) the WASM bindings before Emscripten processes them; that's the --post-js flag given on line 562. that's why the JS code is able to access the symbols in Emscripten's codebase without being exported. but now that the Emscripten build happens before calling await Binaryen(), we need those symbols exported on the object, so they need to be added to EXPORTED_RUNTIME_METHODS. (see the comment in binaryen_js.d.ts
There was a problem hiding this comment.
Can I ask you a question? Are you using Claude to answer me?
There was a problem hiding this comment.
No. Sometimes i do use claude/chatgpt to help me understand code i'm not good at (as a javascript/typescript developer with mostly java experience, i have very little c/c++ experience). And i'll use it to bounce ideas off of when writing new code, but i never take its output at face value. Anything i get from AI i will always review first to make sure it's correct. If/when i use LLM output i will be clear in saying so (as an example see my comment here). Claude has been helpful to me in understanding how Binaryen works, but I did not use it in responding to your comments/questions.
There was a problem hiding this comment.
Using ';', alot of details, links, structural lists and etc.. these are all signs that you're using claude (exept last message). Honestly, it's not just me who's annoyed by this. highly recommnd read this: https://not-an-llm.bearblog.dev/meat-based-llm-proxies
If you don't experienced in C++/emscripten and delegate everything (including this conversion) to Claude or other LLM I'm curious what value does your contribution actually provide? First of all, what value do you personally get out of this?
There was a problem hiding this comment.
i'm being completely honest, everything i'm writing is 100% me. this is just how i write! I like to be explicit and thorough. Semicolons and structured lists make content easier to read as opposed to large blocks of text. I have background in technical writing and content management so that's where i get it from. I don't know how else to convince you.
There was a problem hiding this comment.
speaking freely, i'm a little hurt i'm being accused of this. I'm tired of having to dumb down my writing to avoid coming across as AI. All the things that make written content readable and beautiful: em dashes, numbered lists, block quotes, basically all the diverse punctuation available and elements that Markdown has to offer. We have to stop using them? It's f***ing backwards and it sucks.
I put a lot of work into this project, because i believe it has a lot of potential and i want to enhance the development experience for JS/TS devs. Myself being one of them (that's the value I get out of this, to answer your question @MaxGraey). I'm hoping the team won't let all that go to waste.
|
TODO:
|
The first of several PRs in an attempt to break up #8826 into smaller parts. This PR sets up core typescript functionality and ports over only the top-level types, constants, and enums of the JS API.
Changes:
ts/folder with README.md, package.json, tsconfig.json, and other project filesCMakeLists.txt, exporting needed variables & methods (likeHEAP8) from Emscripten into the JS buildts/src/-pre.tsimports the Emscripten-builtBinaryenfunction, calls and awaits it, and exports that as an internal object calledBinaryenObj(AssemblyScript calls this objectbinaryenand exports that publicly).ts/src/**: the typescript library sits on top of the Emscripten artifact. it accesses the WASM bindings onBinaryenObjimported from-pre.ts.ts/src/binaryen.tsfor users, exporting all the parts of the public-facing API. Users use a namespace import instead of a default import:users will still use the same API (
binaryen.ExpressionRef,binaryen.i32,binaryen.Module, etc.)ts/tests/. hopefully this will take some of the weight off the python testsNon-Changes:
API Deprecations (see
ts/src/-deprecations.tsfor full list; will evolve with more PRs):binaryen.Features→binaryen.Feature)binaryen.Features, but there is a doc-comment@deprecatedwarning for intellisense support)Get started:
See
ts/package.json,ts/README.md, andts/docs/API-Overview.md(in this PR) for details.